Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(cordova/android): fix import_messages script for Android and move it to Node #1674

Merged
merged 4 commits into from
Jul 21, 2023
Merged

chore(cordova/android): fix import_messages script for Android and move it to Node #1674

merged 4 commits into from
Jul 21, 2023

Conversation

sbruens
Copy link
Contributor

@sbruens sbruens commented Jul 21, 2023

No description provided.

@sbruens sbruens requested review from a team as code owners July 21, 2023 17:53
@daniellacosse daniellacosse changed the title chore(cordova/android): Fix import_messages script for Android and move it to Node. chore(cordova/android): fix import_messages script for Android and move it to Node. Jul 21, 2023
@daniellacosse
Copy link
Contributor

Btw I think we can delete the validate_messages.py? I didn't see it used anywhere

@codecov
Copy link

codecov bot commented Jul 21, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: +9 🎉

Comparison is base (317e8f6) 36% compared to head (fd0f6b9) 45%.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #1674    +/-   ##
=======================================
+ Coverage      36%     45%    +9%     
=======================================
  Files          45      39     -6     
  Lines        2756    1943   -813     
  Branches      312     312            
=======================================
- Hits         1006     885   -121     
+ Misses       1750    1058   -692
Flag Coverage Δ
apple ?
ios ?
macos ?
unittests 45% <ø> (+9%) ⬆️
www 45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

daniellacosse
daniellacosse approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@daniellacosse daniellacosse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems fine. Delete that extra python file if we no longer need it. One fewer language for third party contributors to need to know about!

@daniellacosse daniellacosse changed the title chore(cordova/android): fix import_messages script for Android and move it to Node. chore(cordova/android): fix import_messages script for Android and move it to Node Jul 21, 2023
@daniellacosse daniellacosse mentioned this pull request Jul 21, 2023
Closed
@sbruens
Copy link
Contributor Author

sbruens commented Jul 21, 2023

Removed validate_localized_keys.py.

@sbruens sbruens mentioned this pull request Jul 21, 2023
Closed
@sbruens
Copy link
Contributor Author

sbruens commented Jul 21, 2023

For posterity, turns out the reason this broke in #1674 was because of some XML characters (i.e. ') that didn't get properly escaped. Had to do some manual escaping.

@sbruens sbruens merged commit c908e1a into master Jul 21, 2023
12 of 13 checks passed
@sbruens sbruens deleted the sbruens/l10n-node branch July 21, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniellacosse daniellacosse daniellacosse approved these changes

Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sbruens @daniellacosse